London | 26-ITP-January | Eugenie Ahangama | Sprint 3 | Todo List App#1006
London | 26-ITP-January | Eugenie Ahangama | Sprint 3 | Todo List App#1006Eugenie-A wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
| @@ -1,40 +1,50 @@ | |||
| <!DOCTYPE html> | |||
| <!doctype html> | |||
There was a problem hiding this comment.
this is not case sensitive so it doesn't matter but it is more standard to capitalise this.
did your formatter do this automatically?
There was a problem hiding this comment.
Yes, that was my Prettier formatter in VS Code doing it automatically on save. I know it's more standard to keep it uppercase but Prettier lowercases it by default. Prettier formatter is the standard formatter we were asked to use in our work.
| export function deleteCompleted(todos) { | ||
| const completed = todos.filter((todo) => todo.completed); | ||
| completed.forEach((todo) => todos.splice(todos.indexOf(todo), 1)); | ||
| } |
There was a problem hiding this comment.
While this code works, it is inefficient as you are iterating over the entire array when you do filter and then iterating over each of the filtered array items when you do forEach.
The .filter method returns an array filtered by the condition you give it.
Try and see if you can use the .filter method differently to achieve the same outcome, without needing forEach?
There was a problem hiding this comment.
Thank you for the feedback. I've updated the function to use filter once to get the incomplete todos, then splice to update the original array in place.
There was a problem hiding this comment.
Thanks, this works as well.
But again, when you do splice in that second step you're iterating over each of the filtered array items.
This is not necessary.
Try to look at the function again and see if you can do it in 1 line, instead of 2?
There was a problem hiding this comment.
I've combined both lines into one, the filter now happens directly inside the splice call so it's done in a single step.
Learners, PR Template
Self checklist
Changelist
index.htmltodos.mjstodos.test.mjsscript.mjsscript.mjsQuestions
Why is render without parentheses a bug?